Always allow proper serial console on S3/C3/C6/etc under ALL conditions#330
Always allow proper serial console on S3/C3/C6/etc under ALL conditions#330troyhacks wants to merge 3 commits intoMoonModules:mdevfrom
Conversation
📝 WalkthroughWalkthroughAdds ESP32C6 to build-architecture guards and mutual-exclusion checks, expands USB-CDC-on-boot handling to ESP32S3/ESP32C3/ESP32C6, and introduces a CDC-specific boot Serial initialization path that calls Serial.begin(115200) then Serial.setTxTimeoutMs(0). Also broadens runtime USB-CDC gating to include S3/C3/C6. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@wled00/wled.cpp`:
- Around line 470-485: Remove the critical section around
Serial.begin/Serial.setTxTimeoutMs: delete the portMUX_TYPE mux =
portMUX_INITIALIZER_UNLOCKED; portENTER_CRITICAL(&mux); and
portEXIT_CRITICAL(&mux) calls so Serial.begin(115200) and
Serial.setTxTimeoutMs(0) run normally (setup() is single-threaded); if true
exclusivity is required later, replace that pattern with a FreeRTOS recursive
mutex rather than disabling interrupts. Ensure the `#if` branch that currently
contains portENTER_CRITICAL/portEXIT_CRITICAL is simplified to just call
Serial.begin(115200) and Serial.setTxTimeoutMs(0).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@wled00/wled.cpp`:
- Around line 470-475: The build validation currently enumerates allowed
CONFIG_IDF_TARGET_* macros but omits CONFIG_IDF_TARGET_ESP32C6, causing C6
builds to hit the "No supported CONFIG_IDF_TARGET was defined" error; update the
sanity-check macros and any mutual-exclusion guards to include
CONFIG_IDF_TARGET_ESP32C6 (add it alongside CONFIG_IDF_TARGET_ESP32,
CONFIG_IDF_TARGET_ESP32S3, CONFIG_IDF_TARGET_ESP32S2, CONFIG_IDF_TARGET_ESP32C3)
so that the checks that reference these macros (the CONFIG_IDF_TARGET_*
validation block and any `#if/`#elif exclusivity guards) accept ESP32C6 builds and
keep existing behavior for other targets.
|
@troyhacks "/etc" = P4 included? Because i'd like to fix this **** once and for all :-) |
The P4 doesn't have this problem. It has real USB ports for USB things. But in the case of that one Olimex P4 board with a single USB-C port, yes, I think it would fix that too. |
|
It works so well I forgot and built an S3 without it, and it immediately stopped when I disconnected the serial console. It can indeed miss some characters and sometimes looks a bit garbled - but it freakin' works. |
2 participants
This seems to put an and to the clusterfuck that is Serial on the ESP32-S3/C3/C6/etc.
You can now always have a serial console available with:
-D ARDUINO_USB_MODE=1 -D ARDUINO_USB_CDC_ON_BOOT=1 -D ARDUINO_USB_MSC_ON_BOOT=0 -D ARDUINO_USB_DFU_ON_BOOT=0With
WLED_DEBUGyou get a small delay as it tries a Serial few times, even if usingWLEDMM_NO_SERIAL_WAIT- but it still comes up under "only power connected"Plug back into your laptop, and you get a serial console and firmware uploading just the same as an ESP32-reggo.
This ends the madness, I hope.
Summary by CodeRabbit
New Feature/Fix
Bug Fixes